Skip to content

Use full ephemeris data not 5min stat - #150

Merged
taldcroft merged 1 commit into
masterfrom
use-full-telem-for-orbitephem
Nov 25, 2025
Merged

Use full ephemeris data not 5min stat#150
taldcroft merged 1 commit into
masterfrom
use-full-telem-for-orbitephem

Conversation

@taldcroft

@taldcroft taldcroft commented Nov 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This allows running models that depend on the solar / orbit ephemeris files up to the limit of other CXC telemetry on all machines (not just HEAD).

There is an issue with cheta sync where the ephemeris 5min stats can lag behind other telemetry. This impacts GRETA machines and laptops that keep up to date with cheta_sync.

Interface impacts

Models like the ACIS FP model which depend on the ephemeris will change slightly. See below for an example.

Testing

Unit tests

  • Mac
(ska3) ➜  xija git:(use-full-telem-for-orbitephem) git rev-parse --short HEAD                                                     
1066bcb
(ska3) ➜  xija git:(use-full-telem-for-orbitephem) pytest
====================================================== test session starts =======================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 44 items                                                                                                               

xija/tests/test_get_model_spec.py .......                                                                                  [ 15%]
xija/tests/test_models.py .....................................                                                            [100%]

====================================================== 44 passed in 37.49s =======================================================

Independent check of unit tests by Jean

  • OSX
(latest) flame:xija jean$ git rev-parse HEAD
1066bcb1f9097865622092af606ce0035fee87f8
(latest) flame:xija jean$ pytest
========================================================= test session starts ==========================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 45 items                                                                                                                     

xija/tests/test_get_model_spec.py .......                                                                                        [ 15%]
xija/tests/test_models.py ......................................                                                                 [100%]

=========================================================== warnings summary ===========================================================
xija/xija/tests/test_models.py::test_dpa_real[True]
  /Users/jean/miniforge3/envs/latest/lib/python3.12/site-packages/setuptools_scm/git.py:312: UserWarning: git archive did not support describe output
    warnings.warn("git archive did not support describe output")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
==================================================== 45 passed, 1 warning in 38.51

Functional tests

Does it work

At this time, the cheta_sync ephem files go out through 2025:322. Without this PR, the following fails (ValueError: Fetched telemetry does not span model start and stop times for orbitephem0_x), but with the PR it runs as expected.

python -m xija.gui_fit.app acisfp  --stop=2025:324 --days=20

Model impact

For the ACIS FP model, here is a plot of the model difference between this PR and master.
image
image

@taldcroft

taldcroft commented Nov 25, 2025

Copy link
Copy Markdown
Member Author

@jzuhone - There is a good chance this will break the acis_thermal_check regression tests for the ACIS FP model. Do you want to test or just forward-and-fix?

@jeanconn

jeanconn commented Nov 25, 2025

Copy link
Copy Markdown
Contributor

Is the interface impact then that after interpolation there may be small differences in ephemeris data used in the models? I had been thinking the 5 minute averages and 5 minute samples were the same but confess I did not explicitly check.

@taldcroft

taldcroft commented Nov 25, 2025

Copy link
Copy Markdown
Member Author

Yes, I should have noted an interface impact. The ephemeris full data are at exactly 5 minute (300 sec) intervals while the stat data are "5min" 328 sec = 10 major frame intervals. So probably there will be small numerical diffs that show up if regression testing is down to floating point accuracy.

@jeanconn

jeanconn commented Nov 25, 2025

Copy link
Copy Markdown
Contributor

And you hadn't asked for review yet - I just got excited about this one as it seemed a great no-impact fix! Still great if it is a super-tiny-impact fix.

@taldcroft

taldcroft commented Nov 25, 2025

Copy link
Copy Markdown
Member Author

I did some testing and put the results in the description.

@taldcroft
taldcroft merged commit 39c0f58 into master Nov 25, 2025
2 checks passed
@taldcroft
taldcroft deleted the use-full-telem-for-orbitephem branch November 25, 2025 19:53
@taldcroft taldcroft mentioned this pull request Dec 1, 2025
Merged
2 tasks
@javierggt javierggt mentioned this pull request Dec 17, 2025
Closed
This was referenced Jan 20, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeanconn jeanconn jeanconn approved these changes
@jzuhone jzuhone jzuhone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@taldcroft @jeanconn @jzuhone